Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global styles revisions: ensure the revisions endpoint permissions match the global styles controller #50270

Merged
merged 4 commits into from
May 3, 2023

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented May 3, 2023

What

Let's ensure that the endpoint for global styles revisions matches the permissions for the global styles custom post type controller. Huzza!

Why

So that

the endpoint for global styles revisions matches the permissions for the global styles custom post type controller

I'm talking: WP_REST_Global_Styles_Controller->get_item_permissions_check

See #50089 for context

How?

Looking at how we manage permissions for the stored global styles custom post rest controller, and copying it.

Testing Instructions

Run the tests!

npm run test:unit:php -- --filter Gutenberg_REST_Global_Styles_Revisions_Controller_Test

…hes the permissions for the global styles custom post type controller
…t_item_permissions_check, since this is the check against the original global style custom post
@ramonjd ramonjd added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label May 3, 2023
@ramonjd ramonjd self-assigned this May 3, 2023
@ramonjd ramonjd requested a review from spacedmonkey as a code owner May 3, 2023 02:20
@ramonjd ramonjd mentioned this pull request May 3, 2023
4 tasks
if ( ! current_user_can( 'read_post', $post->ID ) ) {
return new WP_Error(
'rest_cannot_view',
__( 'Sorry, you are not allowed to view revisions for this global style.', 'gutenberg' ),
__( 'Sorry, you are not allowed to view this global style.', 'gutenberg' ),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just double-checking, the only intended change in this PR is the wording on this line + adding tests?

In the other PR it looks like a different check was being used. In this case, the current_user_can( 'read_post', $post->ID ) check appears to be consistent with WP_REST_Global_Styles_Controller 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry 🤦

The intention was to ensure the check was exactly the same as WP_REST_Global_Styles_Controller and I copied the translation over as well.

That means the only purpose of this PR is to add 1 x comment and add a test.

Thanks @andrewserong

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, thanks for clarifying! 🙂

@@ -265,6 +265,9 @@ public function get_item_permissions_check( $request ) {
return $post;
}

/*
* The same check as WP_REST_Global_Styles_Controller->get_item_permissions_check.
*/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding comment for context.

Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Confirmed that the check matches that used by WP_REST_Global_Styles_Controller
✅ Tests look good and pass

LGTM! ✨

@andrewserong andrewserong added the [Type] Code Quality Issues or PRs that relate to code quality label May 3, 2023
@ramonjd ramonjd enabled auto-merge (squash) May 3, 2023 04:00
@ramonjd ramonjd merged commit a058a39 into trunk May 3, 2023
@ramonjd ramonjd deleted the update/global-styles-revisions-rest-permissions branch May 3, 2023 04:29
@github-actions github-actions bot added this to the Gutenberg 15.8 milestone May 3, 2023
@ramonjd ramonjd added the Needs PHP backport Needs PHP backport to Core label Jun 5, 2023
@ramonjd ramonjd removed the Needs PHP backport Needs PHP backport to Core label Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants